-
Notifications
You must be signed in to change notification settings - Fork 593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
wallet: Return an error if select utxos in txtoOutputs
contain duplicates.
#928
Conversation
|
wallet/createtx.go
Outdated
@@ -179,6 +180,10 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, | |||
|
|||
var inputSource txauthor.InputSource | |||
if len(selectedUtxos) > 0 { | |||
if !fn.IsUniqueVals(selectedUtxos) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to just inline this function here, since it's literally a single line, instead of waiting for another PR to be merged and tag to be pushed.
0401a41
to
7bb8bbb
Compare
wallet/createtx.go
Outdated
@@ -179,6 +180,12 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, | |||
|
|||
var inputSource txauthor.InputSource | |||
if len(selectedUtxos) > 0 { | |||
if len(fn.NewSet(selectedUtxos...)) != |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: (if line length permits)
dedupUtxos := fn.NewSet(selectedUtxos...)
if len(dedupUtxos) != len(selectedUtxos) {
wallet/createtx_test.go
Outdated
} | ||
tx1, err := w.txToOutputs( | ||
[]*wire.TxOut{targetTxOut}, nil, nil, 0, 1, 1000, | ||
CoinSelectionLargest, true, tc.selectUtxos, alwaysAllowUtxo, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please set up your IDE in a way that you get a visual indication of where the 80 characters cutoff limit is (and setting the tabulator width correctly too)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies about this, as I just copied and put it in a for block, I did not recheck for the effect on the line length.
8606565
to
e42363c
Compare
Signed-off-by: Ononiwu Maureen <[email protected]>
Signed-off-by: Ononiwu Maureen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
cc @sputn1ck, can't request review through GitHub. |
Signed-off-by: Ononiwu Maureen <[email protected]>
Replaced by #969 (commit author attribution preserved). |
As detailed here, the sendcoins command in lnd does not fail if a user supplies duplicate select utxos:
lightningnetwork/lnd#8516 (comment)
This means that we could potentially end up crafting invalid transactions as a result of this bug. This PR fixes that by returning an error in such scenarios. Testcases are also added to test for this.